Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ADR 009: InfoReservedByte #843

Closed

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Aug 22, 2022

Description

Still a work in progress. In particular, I will add more details to the implementation details section

@rootulp rootulp requested a review from evan-forbes August 22, 2022 20:32
@rootulp rootulp self-assigned this Aug 22, 2022
@musalbas musalbas self-requested a review August 22, 2022 22:41
Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! the only thing left is to add it when splitting shares! simple pimple. well done

docs/celestia-architecture/adr-009-info-reserved-byte.md Outdated Show resolved Hide resolved
@evan-forbes
Copy link
Member

also, do we want to move this to the app, considering that's where all the logic will be and this ADR modifies application specific types?

@rootulp
Copy link
Collaborator Author

rootulp commented Aug 23, 2022

nice! the only thing left is to add it when splitting shares! simple pimple. well done

Added in 26884d1 . Resolving #842 and merging celestiaorg/celestia-app#637 would make implementing this ADR significantly easier to tackle.

@rootulp
Copy link
Collaborator Author

rootulp commented Aug 23, 2022

also, do we want to move this to the app, considering that's where all the logic will be and this ADR modifies application specific types?

Agreed. I originally targeted celestia-core because all the relevant issues are in this repo. Resolving this ADR will involve modifying celestia-core types too (e.g. Message struct) but will move to celestia-app because that's where share logic will live long term.

@rootulp rootulp closed this Aug 23, 2022
@rootulp rootulp deleted the rp/adr-info-reserved-byte branch August 23, 2022 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants